Skip to content

feat: compile pipeline, query agent, and multimodal improvements#10

Merged
rejojer merged 12 commits into
devfrom
bugfix/compile-clean
Apr 10, 2026
Merged

feat: compile pipeline, query agent, and multimodal improvements#10
rejojer merged 12 commits into
devfrom
bugfix/compile-clean

Conversation

@rejojer

@rejojer rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member

Summary

  • Concept dedup & compile refactor: concept plan/update/related paths, bidirectional backlinks, shared _compile_concepts
  • Brief system & unified query: per-page JSON sources via pymupdf, doc_type + full_text frontmatter, get_page_content tool
  • Config & environment: default model gpt-5.4-mini, API key warning, warning suppression, test isolation
  • CLI polish: improved init prompts with explicit defaults, American English, output formatting
  • Multimodal query: get_image tool with ToolOutputImage for viewing figures/charts in source documents
  • Image path unification: all image paths use sources/images/ prefix, pymupdf replaces PageIndex for page content extraction

Test plan

  • All 184 tests pass
  • openkb init shows correct prompts with defaults
  • openkb add short doc: clean frontmatter, images at sources/images/
  • openkb add long doc: per-page JSON from pymupdf, correct image paths
  • openkb query on short doc: reads source via read_file
  • openkb query on long doc: uses get_page_content with targeted pages
  • openkb query about figures: calls get_image tool

KylinMountain and others added 6 commits April 10, 2026 08:04
…klinks

- Add concept dedup with briefs and _read_concept_briefs context
- Add concepts plan and update prompt templates with create/update/related paths
- Extract shared _compile_concepts from compile_short_doc and compile_long_doc
- Add bidirectional backlinks between summaries and concepts
- Code review fixes: security, robustness, tests, and CI hardening

Co-authored-by: Ray <mailtangyu@gmail.com>
- Add get_page_content tool and parse_pages helper for page-level access
- Store long doc sources as per-page JSON extracted by pymupdf
- Unify summary frontmatter to doc_type + full_text fields
- Update schema and tree renderer for new frontmatter format
- All image paths use sources/images/ prefix relative to wiki root

Co-authored-by: Ray <mailtangyu@gmail.com>
- Change default model to gpt-5.4-mini
- Warn when no LLM API key found instead of failing silently
- Fix CI publish workflow and test isolation

Co-authored-by: Ray <mailtangyu@gmail.com>
- Move warning suppression after imports to avoid markitdown override
- Improve init prompts with explicit defaults
- Use American English throughout (initialized, normalized, Synthesize)
- Replace unicode ellipsis with ASCII
- Remove empty explorations/reports dirs from init
- Fix test isolation for _find_kb_dir
- Add get_image tool for viewing images referenced in source documents
- Use ToolOutputImage for proper image content in LLM context
- Update prompt: use full_text field, restrict get_page_content to pageindex
- Add self-talk before tool calls, enforce concise answers
- Prevent duplicate frontmatter in LLM-generated content via schema update
- Add convert_pdf_to_pages for per-page content+image extraction
- All image paths use sources/images/ prefix relative to wiki root
- Remove page marker comments from short doc source markdown
@rejojer
rejojer force-pushed the bugfix/compile-clean branch from 9f652d0 to 44bf83e Compare April 10, 2026 00:09
@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept appends LLM-rewritten content instead of replacing the body during concept updates, causing content duplication. The _CONCEPT_UPDATE_USER prompt (line 106) instructs the LLM to "Rewrite the full page incorporating the new information naturally -- do not just append" and return "The rewritten full concept page." However, _write_concept appends the complete rewrite to the existing body (existing += f"\n\n{clean}" at line 341) rather than replacing the body with it. Every concept update with a new source document will duplicate all existing content.

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
existing += f"\n\n{clean}"
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

rejojer added 5 commits April 10, 2026 08:33
The _CONCEPT_UPDATE_USER prompt asks the LLM for a full rewrite, but
_write_concept was appending the rewrite to the existing body, causing
content duplication on every concept update.
Replace hand-rolled fence stripping with json_repair to handle
malformed JSON, missing fences, and prose-wrapped responses from LLMs.
Also fixes str.index() ValueError on fenced blocks without newlines.
@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept silently discards LLM-generated content on re-compilation of the same document. When is_update=True and the concept page already exists, the body-replacement logic (lines 339-353) is entirely nested inside if source_file not in existing:. On re-compile, source_file is already present in the frontmatter, so this condition is false and the LLM rewrite is silently dropped -- the API call is made and paid for, but the result is never written. Only the brief field gets updated (lines 354-363, outside the gate). The source_file not in existing check should only guard the "add source to frontmatter" block, not the body replacement.

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
# Replace body with LLM rewrite (prompt asks for full rewrite, not delta)
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
existing = existing[:end + 3] + "\n\n" + clean
else:
existing = clean
else:
existing = clean
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept silently discards LLM-generated concept rewrites on re-compilation. When is_update=True and source_file already appears in the existing concept page (in sources: [...] frontmatter), the source_file not in existing guard at line 326 causes the entire body-replacement block (lines 327-353) to be skipped. Only the brief frontmatter field is updated; the full page rewrite from _CONCEPT_UPDATE_USER is thrown away. This means re-adding a modified document leaves concept pages stale. (bug: body replacement wrongly gated inside source-dedup check)

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
# Replace body with LLM rewrite (prompt asks for full rewrite, not delta)
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
existing = existing[:end + 3] + "\n\n" + clean
else:
existing = clean
else:
existing = clean
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@rejojer
rejojer merged commit 85eaebf into dev Apr 10, 2026
@rejojer rejojer mentioned this pull request Apr 10, 2026
4 tasks
rejojer added a commit that referenced this pull request Apr 11, 2026
feat: compile pipeline, query agent, and multimodal improvements
@rejojer
rejojer deleted the bugfix/compile-clean branch April 11, 2026 17:45
KylinMountain added a commit that referenced this pull request Jul 21, 2026
…es (#198)

* feat(api): delete a knowledge base (POST /api/v1/kb/delete + openkb delete-kb)

Physical, irreversible KB deletion with a type-the-name confirmation.

- config.delete_kb: rmtree the KB directory + unregister it from the global
  registry (known_kbs / kb_aliases / default_kb, via the new unregister_kb).
  Guards against deleting a non-KB path; tolerates a ghost registry entry
  (directory already gone) by just unregistering.
- POST /api/v1/kb/delete: confirm_name must equal kb, re-checked server-side.
  Lives in a new api_kbs_router (sibling of the config/graph/output routers) so
  api.py stays under the 800-line gate; delete_kb self-locks (config lock), no
  create_app closure needed.
- openkb delete-kb NAME: type-the-name prompt (or --yes) then delete.

Tests: endpoint (success + unregister, confirm mismatch, non-KB target) in
test_api.py; config (physical delete + unregister, refuse non-KB, ghost
tolerance) in test_config.py.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* feat(api): delete a wiki page (POST /api/v1/page/delete) with backlink impact

Delete a concept/entity page and degrade references cleanly instead of leaving
them dangling:

- page_ops.delete_wiki_page: dry_run reports the backlink pages (whose inbound
  [[wikilinks]] would be demoted) without touching anything; execute removes the
  page under the KB ingest lock, strips its index.md entry outright
  (compiler.remove_doc_from_index — the entry is a [[link]] line), and demotes
  the now-dangling inbound [[links]] to plain text in ONLY those backlink pages
  (lint.fix_broken_links restrict_to). The page ref is "section/stem", validated
  to concepts/ or entities/ only (path-traversal-safe).
- New api_pages_router hosts POST /api/v1/page (read — relocated here from
  api.py to keep it under the 800-line gate) + POST /api/v1/page/delete.

Tests: dry-run impact + execute (page gone, inbound link demoted to text, index
entry removed, sibling kept), 404 on missing page, and rejection of unsafe /
non-editable refs.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* feat(api): edit a wiki page (PUT /api/v1/page) + link context (POST /api/v1/page/links)

- page_ops.edit_wiki_page: replace a concept/entity page's BODY while preserving
  its code-managed OKF frontmatter (type/description/sources) verbatim; any
  frontmatter in the submitted content is dropped. Dead [[wikilinks]] the user
  typed are demoted to plain text on save (OpenKB keeps the wiki broken-link-free)
  and returned as ghosts_stripped so the UI can warn. Atomic write under the KB
  ingest lock.
- page_ops.page_link_context (POST /api/v1/page/links): outbound + inbound links
  for the edit-impact panel. Editing the body does not break either (links are
  path-based), so this is context, not a blocker — the honest impact model.
- PUT /api/v1/page added to api_pages_router.

Tests: edit preserves frontmatter + keeps a resolvable link + demotes a dead one
(reports ghosts_stripped) + replaces the body; 404 on a missing page; links
reports out/backlinks.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* feat(web): delete a knowledge base from the settings sheet (type-name confirm)

Adds a Danger Zone section to KbSettingsSheet: a type-the-name confirmation
gates deleteKb() (POST /api/v1/kb/delete); on success KbDetail navigates back
to the KB list. New deleteKb client + kbSettings danger* keys + common cancel
(zh + en, identical key sets).

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* feat(web): in-reader page edit + delete with impact preview (F2/F3)

For an open concept/entity page, the reader header now shows Edit and Delete:
- Delete: a dry-run first (deletePage dryRun) surfaces the backlink pages whose
  [[links]] will demote to plain text; a red confirm card lists them, then the
  real delete closes the page and refreshes the inventory.
- Edit: a body textarea (frontmatter is preserved server-side), Save via
  PUT /api/v1/page, a links panel (getPageLinks: out/backlinks), a "recompile
  may overwrite" note, and a toast listing any dead links demoted to text.

Adds the deletePage/editPage/getPageLinks wiki clients and kb:pageOps.* locale
keys (zh + en, identical key sets). Build green (i18n guard + tsc + vite).

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* fix(workbench): address code-review findings (content-mgmt correctness + concurrency)

Backend (page_ops / kb_admin / lint):
- delete/edit now do their read-modify-write INSIDE the KB ingest lock and
  re-check the page exists under it: no stale backlink snapshot, no resurrecting
  a concurrently-deleted page, no 500 on a racing unlink (missing_ok=True). [#4,#5,#6]
- delete_kb serializes rmtree under the ingest lock (was fully unlocked). Moved
  delete_kb/unregister_kb into a new openkb/kb_admin.py so config.py drops back
  under the per-file line gate (751 lines). [#1,#15]
- page deletion no longer scans or rewrites lint's _EXCLUDED_FILES
  (AGENTS.md/SCHEMA.md/log.md); fix_broken_links(restrict_to) also refuses them,
  which protects `openkb remove` too. [#2]
- edit_wiki_page treats `content` as the body verbatim — no naive frontmatter
  split that silently dropped a body opening with a '---' block. [#3]
- delete uses the real on-disk stem for the exact-match index-entry removal
  (case-insensitive FS), and adds index.md to the demotion set so a [[target]]
  embedded in another entry's brief no longer dangles. [#7,#9]

API / CLI:
- delete-KB endpoint & CLI can now clean a ghost registry entry (registered name
  whose directory was removed by hand), catch delete_kb's ValueError -> 400, and
  the endpoint is typed `-> KbDeleteResponse`. [#8,#13,E5]

Frontend (KbDetail):
- Deleting a KB dispatches `openkb:reload-kbs` so the sidebar refreshes
  immediately (was stale until a manual reload) — live-test fix.
- Removed the dead `pageReloadSeq` path (editPage always returns content). [#14]

Tests: excluded-doc-untouched, '---'-body preservation, ghost-KB unregister;
non-KB-target test updated to the new resolution model. Backend 1234 passed,
mypy/ruff clean, frontend build green.

Not changed (intentional): pages_linking_to is NOT folded into build_graph (that
excludes explorations/ backlinks) [#10]; the preview+confirm delete inherently
scans per request [#11]; EDITABLE_SECTIONS stays duplicated in the frontend,
which can't import the Python constant [#12].

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* feat(workbench): allow editing summary pages (edit ⊇ delete section sets)

Summaries are compiled markdown like concepts/entities (a recompile regenerates
them), so they are now editable via PUT /api/v1/page. They remain NOT
independently deletable — a summary is removed by deleting its source document.

Split page_ops into EDITABLE_SECTIONS (concepts/entities/summaries) vs
DELETABLE_SECTIONS (concepts/entities); validate_page_ref takes the allowlist.
Frontend ReaderBody gates Edit on canEdit (incl. summaries), Delete on canDelete.

Test: summary editable (frontmatter preserved) + summary delete rejected (400).

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

* fix(api): cross-platform delete_kb + endpoint OSError handling (xhigh review #1,#2)

- delete_kb no longer holds the ingest lock DURING rmtree (the lock file lives
  inside the KB dir; Windows cannot delete an open file — the prior review-fix
  regressed this). It now takes the lock as a BARRIER (drain + wait out any
  in-flight mutation), releases it, re-checks existence, then rmtrees. [#1]
- delete-KB endpoint maps FileNotFoundError to an idempotent success (a
  concurrent delete already removed the tree) and other OSError to a clean 500
  with a message, instead of an uncaught 500 stack trace. [#2]

Tests: endpoint OSError -> clean 500, FileNotFoundError -> 200 deleted.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants